getHelperFtn support for getting CORINFO_METHOD_HANDLE by davidwrighton · Pull Request #116603 · dotnet/runtime

davidwrighton · 2025-06-12T20:27:53Z

If there is one associated, we will get the method handle. This is an optional parameter, as is the logic to get the existing result.
This will be used in the interpreter to generate different bytecode that knows about calling managed methods when needed
There is also some prior interest from the JIT team about using such an api, but it didn't quite get in. See PR Add pMethod param to getHelperFtn EE-JIT API #110267. This PR takes the comment from @jkotas into account to adjust the signature of the method to be something which is simpler and easier to understand

@jkotas

- If there is one associated, we will get the method handle. This is an optional parameter, as is the logic to get the existing result. - This will be used in the interpreter to generate different bytecode that knows about calling managed methods when needed - There is also some prior interest from the JIT team about using such an api, but it didn't quite get in. See PR dotnet#110267. This PR takes the comment from @jkotas into account to adjust the signature of the method to be something which is simpler and easier to understand

Copilot

Pull Request Overview

This PR updates the JIT interface helper functions to support retrieving the CORINFO_METHOD_HANDLE and native entry point via a new API signature. Key changes include:

Changing the getHelperFtn signature in several headers and implementations from returning a void* to using output parameters (CORINFO_CONST_LOOKUP and CORINFO_METHOD_HANDLE).
Propagating the new API usage across various JIT components and code generators.
Adjusting call sites to handle the new lookup structure and access type semantics.

Reviewed Changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/coreclr/tools/aot/jitinterface/jitinterface_generated.h	Updated getHelperFtn signature to include native entrypoint and method handle.
src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt	Modified helper function parameter types to use REF_CORINFO_CONST_LOOKUP.
src/coreclr/jit/CorInfoImpl_generated.cs	Adjusted managed wrapper for getHelperFtn to match new signature.
src/coreclr/jit/CorInfoImpl.cs	Updated managed implementation of getHelperFtn using the new API.
src/coreclr/jit/lower.cpp, importer.cpp, compiler.*	Updated call sites for helper functions to use CORINFO_CONST_LOOKUP instead of raw pointers.
src/coreclr/jit/codegen.	Updated helper lookup usage across various code generators.
src/coreclr/inc/*	Updated declarations for getHelperFtn in the EE interface definition.

Copilot · 2025-06-12T20:28:43Z

+        {
+            params.addr = nullptr;
+            assert(helperFunction.accessType == IAT_PVALUE);
+            void* pAddr = helperFunction.addr;


The variable 'pAddr' is declared in the else block but never used. Removing this redundant declaration would improve code clarity.

Copilot · 2025-06-12T20:28:44Z

+    {
+        params.addr = nullptr;
+        assert(helperFunction.accessType == IAT_PVALUE);
+        void* pAddr = helperFunction.addr;


The redundant declaration of 'pAddr' in the else branch is not used; consider removing it to simplify the code.

Suggested change

void* pAddr = helperFunction.addr;

kg · 2025-06-12T20:46:22Z

        // If we don't have a matched VM, we won't get valid results when asking for a helper function.
-        addr = (void*)(uintptr_t)(0xCA11CA11); // "callcall"
+        lookup.addr = (void*)(uintptr_t)(0xCA11CA11); // "callcall"
+        lookup.accessType = IAT_VALUE;


should we add a new IAT_ enum for 'nothing' so that anything that switch()es on the accessType will fail fast?

kg · 2025-06-12T20:49:18Z


        [UnmanagedCallersOnly]
-        private static void* _getHelperFtn(IntPtr thisHandle, IntPtr* ppException, CorInfoHelpFunc ftnNum, void** ppIndirection)
+        private static void _getHelperFtn(IntPtr thisHandle, IntPtr* ppException, CorInfoHelpFunc ftnNum, CORINFO_CONST_LOOKUP* pNativeEntrypoint, CORINFO_METHOD_STRUCT_** pMethod)


this makes me wonder whether it should be called ppMethod instead

No, not really... a method is a CORINFO_METHOD_STRUCT_* ... the first pointer is just part of being a method. This is just messy interop things, since we couldn't use a normal struct here to represent a typedef, since pointers and structs that only have a pointer in them are represented differently in the native ABI on some platforms. @max-charlamb recently talked to @jkoritzinsky and we now have an actual clean way to represent this using the p/invoke source generator (this is the CustomMarshalerAttribute support), but this logic predates that feature by several years.

Yeah at some point we should be able to move to source-gen interop for this. Would require us to productize our "vtables but not COM" generator though (or make the JIT-EE interface build off of IUnknown just enough for the COM generator to interface well).

EgorBo · 2025-06-12T20:55:37Z

    Packet_NotifyInstructionSetUsage = 229,
    Packet_GetAsyncInfo = 230,
    Packet_GetAsyncResumptionStub = 231,
+    Packet_GetHelperFtn = 233,


I don't think this is needed?

My understanding is that if you change the size of contents of the data packet in SPMI you need a new magic number. See MethodContext::MethodInitHelper. Since this PR changes the definition of what the LWM map for GetHelperFtn is, I updated the magic value.

It's not needed -- the JIT-EE GUID update is sufficient.

It would also be nice to fix the wrong type added in #116449 (comment) (since you're going to have to resolve merge conflicts anyway...)

kg · 2025-06-12T21:07:35Z

LGTM overall

EgorBo · 2025-06-12T21:41:22Z

Bumping JIT-EE guid is hot these days 🙂

dotnet-policy-service · 2025-06-12T21:42:39Z

Tagging @dotnet/jit-contrib for JIT-EE GUID update

…e_to_gethelperftn

jkotas · 2025-06-12T23:26:28Z

-            else
+            // We never return a method handle from the managed implementation of this method today
+            if (pMethod != null)
+                *pMethod = null;


We will want to fix this if/once JIT starts doing helper inlining. cc @EgorBo

risc-vv · 2025-06-13T08:51:00Z

a6513e3 is being scheduled for building and testing

GIT: a6513e36266ea31f26c432b37f515f944f151418
REPO: dotnet/runtime
BRANCH: main

sirntar · 2025-06-13T09:27:49Z

@dotnet/samsung

davidwrighton · 2025-06-13T17:42:15Z

/ba-g these failures are known and unrelated

davidwrighton requested review from EgorBo, Copilot and jkotas June 12, 2025 20:27

davidwrighton requested review from BrzVlad, MichalStrehovsky, janvorli and kg as code owners June 12, 2025 20:27

github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 12, 2025

dotnet-policy-service Bot assigned davidwrighton Jun 12, 2025

Copilot AI reviewed Jun 12, 2025

View reviewed changes

JIT formatting

cc8b66b

kg reviewed Jun 12, 2025

View reviewed changes

EgorBo reviewed Jun 12, 2025

View reviewed changes

Comment thread src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp

EgorBo reviewed Jun 12, 2025

View reviewed changes

jkotas mentioned this pull request Jun 12, 2025

Add pMethod param to getHelperFtn EE-JIT API #110267

Closed

3 tasks

EgorBo approved these changes Jun 12, 2025

View reviewed changes

Attempt to address build breaks

6c21cd2

janvorli approved these changes Jun 12, 2025

View reviewed changes

davidwrighton added 2 commits June 12, 2025 15:26

Merge branch 'main' of github.com:dotnet/runtime into add_methodhandl…

12ef1a5

…e_to_gethelperftn

Fix build break

a6513e3

davidwrighton enabled auto-merge (squash) June 12, 2025 23:09

jkotas reviewed Jun 12, 2025

View reviewed changes

davidwrighton merged commit 4623289 into dotnet:main Jun 13, 2025
2 checks passed

github-actions Bot locked and limited conversation to collaborators Jul 14, 2025

Uh oh!

Conversation

davidwrighton commented Jun 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kg commented Jun 12, 2025

Uh oh!

EgorBo commented Jun 12, 2025

Uh oh!

dotnet-policy-service Bot commented Jun 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

risc-vv commented Jun 13, 2025

Uh oh!

sirntar commented Jun 13, 2025

Uh oh!

davidwrighton commented Jun 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants